Conversation
demologin
left a comment
demologin
left a comment
There was a problem hiding this comment.
Общий вывод по проекту
Проект демонстрирует хорошее владение базовыми принципами Java и архитектурными паттернами. Использование Command pattern для обработки запросов и Mockito для тестов — это отличный уровень. Однако стоит уделить больше внимания безопасности (валидация входных данных) и чистоте кода (Dependency Injection вместо поиска в репозитории объектов).
Итоговая оценка: A
|
|
||
| private int getSelectedQuestIndex(HttpServletRequest req) { | ||
| String questIndexString = req.getParameter(Parameter.QUEST_INDEX); | ||
| return Integer.parseInt(questIndexString); |
There was a problem hiding this comment.
Метод Integer.parseInt может выбросить NumberFormatException, если параметр отсутствует или не является числом. Следует добавить валидацию входных данных. [ERROR]
|
|
||
| private int getSelectedNextNodeId(HttpServletRequest req) { | ||
| String nextNodeIdString = req.getParameter(Parameter.NEXT_NODE_ID); | ||
| return Integer.parseInt(nextNodeIdString); |
There was a problem hiding this comment.
Прямое приведение строки из параметров запроса к числу без проверки на null или формат является потенциальным источником RuntimeException. [ERROR]
| ObjectMapper mapper = ObjectRepository.find(ObjectMapper.class); | ||
| Quest quest = mapper.readValue(req.getReader(), Quest.class); | ||
| QuestService questService = ObjectRepository.find(QuestService.class); | ||
| questService.saveQuest(quest); |
There was a problem hiding this comment.
Результат сохранения квеста игнорируется. Если saveQuest возвращает путь к файлу, возможно, его стоит использовать для логирования или информирования пользователя. [INFO]
| QuestService questService = ObjectRepository.find(QuestService.class); | ||
| questService.saveQuest(quest); | ||
| } catch (IOException e) { | ||
| throw new RuntimeException(e); |
There was a problem hiding this comment.
Оборачивание IOException в RuntimeException без контекста затрудняет отладку. Рекомендуется использовать пользовательские исключения или информативные сообщения. [WARNING]
| default String doPost(HttpServletRequest request) { | ||
| public String doPost(HttpServletRequest req) { | ||
| return getView(); | ||
| } |
There was a problem hiding this comment.
Логика преобразования CamelCase в kebab-style может быть упрощена с использованием регулярных выражений Java 21. [INFO]
| @@ -32,6 +33,4 @@ private static String convertCamelCaseToKebabStyle(String string) { | |||
| ? snakeName.substring(1) | |||
There was a problem hiding this comment.
Вспомогательный метод преобразования строк стоит вынести в отдельный утилитный класс, чтобы не загромождать базовый абстрактный класс. [INFO]
| String view = nextStage.doGet(req, servlet); | ||
|
|
||
| assertEquals(Go.RESULT, view); | ||
| assertEquals(1, statistics.getVictoriesCount()); |
There was a problem hiding this comment.
Тест проверяет сразу несколько сценариев (победа, поражение, обычный ход) в одном методе. Стоит разделить на разные тестовые методы. [INFO]
| try { | ||
| req.setCharacterEncoding("UTF-8"); | ||
| ObjectMapper mapper = ObjectRepository.find(ObjectMapper.class); | ||
| Quest quest = mapper.readValue(req.getReader(), Quest.class); |
There was a problem hiding this comment.
Чтение всего тела запроса через getReader() и маппинг в объект может упасть с ошибкой памяти при очень больших файлах квестов. [WARNING]
| public class MainMenu extends Command { | ||
|
|
||
| @Override | ||
| public String doGet(HttpServletRequest req, HttpServlet servlet) { |
There was a problem hiding this comment.
Метод не содержит логирования. Для Senior уровня важно отслеживать действия пользователя (например, вход в главное меню). [INFO]
| } | ||
|
|
||
| private int getSelectedNextNodeId(HttpServletRequest req) { | ||
| String nextNodeIdString = req.getParameter(Parameter.NEXT_NODE_ID); |
There was a problem hiding this comment.
Имя параметра NEXT_NODE_ID лучше вынести в константу класса Parameter для исключения опечаток. [INFO]
2 participants
No description provided.